Constructive Code Reviews

2019-12-21

These are some guidelines for code reviews that I like to follow. Hopefully this will be a list that I keep refining and adding to as I gain more experience.

The main theme behind these guidelines is to remember that there is a person behind the screen. It is easy to forget this; an e-mail pops in your inbox, and there it is, the code review. Just in an instant, out of nowhere. But this sense of speed is false, a product of machines, and not representative of all the human effort that goes into code. Remember that there is a person on the other end, and communicate to leave a positive impact on them for their own good as well as yours.

Guidelines

Positive grammar

Structure your grammar to leave a positive connotation. In a compound sentence, typically the thing I remember more strongly is the last clause, so I make sure to end my sentences in a positive tone.

For example, suppose that someone tried to fix the car, but the car remains broken despite their attempts to fix it. We might say something like:

You tried to fix the car, but it’s still broken.

The car is still broken, although you tried to fix it.

The fact is the same, the difference is in connotation. In the first sentence, the thing I will remember the most is “it’s still broken”, which has a negative connotation and seems to punish the person for not fixing the car. In the second sentence, the last and more memorable clause is “although you tried to fix it”, which has a more positive connotation and recognizes the person for their effort in at least trying to tackle the problem. Make the positive clause the last one.

Propose alternatives

Don’t just say you don’t like something. It’s annoying, and does not add anything to the conversation. For everything you are not entirely happy with, try to propose a better alternative. This is possible most of the times. Other times, the problem might truly be complex and you might be unable to think of an alternative. In that case, explicitly state that you tried to find one such alternative and could not come up with one, and maybe suggest a follow-up discussion. Don’t leave your comments open-ended and leave the other person wondering.

Find something positive

There is almost always something positive in the code review. Identify that part, and recognize the person for their effort. Adding such kind of non-actionable comment on the review might seem repetitive and unnecessary, but it is only a minor annoyance compared to the sense of appreciation the other person will get.

Reserving “I”

I tend to avoid “I” and use “we” instead. “We” seems to reinforce the sense of teamwork, as in “we” are working on this solution together. If I use “I”, it is only in the rare case where I may express a personal opinion, or when I am instructing the other person to do something; “I suggest reading XYZ at…”. Most of the times, “we” is the better choice.

Be thankful

It might take you 20-30 minutes to review the code. It might have taken the other person several hours or even days to write it. This is especially true of simple ideas, which tend to be expressed succinctly in code and are not representative of all the work that went into them. So be thankful for the other person’s effort, and explicitly recognize it by saying thanks. This can be done in individual comments of the review or in the general comments section.

Beware of tone, or lack thereof

Much of the tone gets lost in writing, especially for those for which English is not their native language (and if you are in an international team, this will almost always be the case.) Remember, then, that the way in which you interpret a written message may not be the same way the other person intended to convey it. Be cognizant of this, stay focused on content, and hash out any misunderstandings in person if necessary.